Skip to content

feat: Update streaming chunk#9424

Merged
sjrl merged 37 commits intomainfrom
update-streaming-chunk
Jun 6, 2025
Merged

feat: Update streaming chunk#9424
sjrl merged 37 commits intomainfrom
update-streaming-chunk

Conversation

@sjrl
Copy link
Copy Markdown
Contributor

@sjrl sjrl commented May 22, 2025 •
edited
Loading

Related Issues

Proposed Changes:

Updated StreamingChunk to add the fields tool_call, tool_call_result, index, and start to make it easier to format the stream in a streaming callback.

  • Added new dataclass ToolCallDelta for the StreamingChunk.tool_call field to reflect that the arguments can be a string delta.
  • Updated print_streaming_chunk and _convert_streaming_chunks_to_chat_message utility methods to use these new fields. This especially improves the formatting when using print_streaming_chunk with Agent.
  • Updated OpenAIGenerator, OpenAIChatGenerator, HuggingFaceAPIGenerator, HuggingFaceAPIChatGenerator, HuggingFaceLocalGenerator and HuggingFaceLocalChatGenerator to follow the new dataclasses.
  • Updated ToolInvoker to follow the StreamingChunk dataclass.

How did you test it?

  • Added unit tests
  • Tested Agent streaming locally with OpenAIChatGenerator

Before Changes
Screenshot 2025-05-22 at 13 35 23

After Changes

  • Notable differences
    • Possible to add the [ASSISTANT] header
    • Easier to have better spacing between sections (e.g. between two tool calls)
Screenshot 2025-05-22 at 13 34 11

Notes for the reviewer

Just as a heads up ~450 lines of code change from the tests so don't bee too alarmed by the large PR.

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@github-actions github-actions Bot added topic:tests type:documentation Improvements on the docs labels May 22, 2025
sjrl
sjrl commented May 22, 2025
View reviewed changes
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
sjrl added 2 commits May 22, 2025 12:13
@sjrl
Properly update test
e29b6f2
@sjrl
Change to tool_name, remove kw_only since its python 3.10 only and up…
5914d5b 
…date HuggingFaceAPIChatGenerator to start following new StreamingChunk
sjrl
sjrl commented May 22, 2025
View reviewed changes
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
Comment on lines +50 to +51
tool_call: Optional[ToolCallDelta] = None
tool_call_result: Optional[ToolCallResult] = None
Copy link
Copy Markdown
Contributor Author

@sjrl sjrl May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about making

content = Union[str, ToolCallDelta, ToolCallResult]

but this would be a breaking change b/c users expect content to always be a string. And this breaks StreamingChunk to ChatMessage implementations (mostly in private methods).

@sjrl sjrl requested a review from tstadel May 22, 2025 11:44
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented May 22, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 15485894448

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on update-streaming-chunk at 90.418%
Totals Coverage Status
Change from base Build 15414807243: 90.4%
Covered Lines: 11512
Relevant Lines: 12732
💛 - Coveralls

@sjrl sjrl mentioned this pull request May 23, 2025
Merged
@tstadel
Copy link
Copy Markdown
Member

tstadel commented May 23, 2025

Looks very good to me already. 👍

sjrl
sjrl commented Jun 2, 2025
View reviewed changes
Comment thread haystack/components/tools/tool_invoker.py
Comment on lines +507 to +509
index=len(tool_messages) - 1,
tool_call_result=tool_messages[-1].tool_call_results[0],
start=True,
Copy link
Copy Markdown
Contributor Author

@sjrl sjrl Jun 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ToolInvoker the whole ToolCallResult is always sent so start is always True and index is just the current message index.

sjrl
sjrl commented Jun 2, 2025
View reviewed changes
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
@sjrl
Copy link
Copy Markdown
Contributor Author

sjrl commented Jun 2, 2025

Failing tests are fixed with this PR #9457

@sjrl sjrl marked this pull request as ready for review June 2, 2025 07:59
@sjrl sjrl requested review from a team as code owners June 2, 2025 07:59
@sjrl sjrl requested review from dfokina and julian-risch and removed request for a team June 2, 2025 07:59
julian-risch
julian-risch requested changes Jun 4, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite good to me already! 👍 I have only minor comments and regarding Optional[bool] for start in StreamingChunk, I would like to better understand what the meaning of None would be.

Comment thread haystack/components/generators/chat/openai.py Outdated
Comment thread haystack/components/generators/chat/openai.py Outdated
Comment thread haystack/components/generators/openai.py
Comment thread haystack/components/generators/utils.py Outdated
Comment thread haystack/components/generators/utils.py Outdated
Comment thread test/components/generators/chat/test_openai.py
Comment thread haystack/utils/hf.py Outdated
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
Comment thread haystack/dataclasses/__init__.py
Comment thread haystack/dataclasses/streaming_chunk.py Outdated
sjrl and others added 4 commits June 4, 2025 12:18
@sjrl @julian-risch
Update haystack/components/generators/utils.py
06cb3ec 
Co-authored-by: Julian Risch <julian.risch@deepset.ai>
@sjrl @julian-risch
Update haystack/components/generators/utils.py
c2a2eb0 
Co-authored-by: Julian Risch <julian.risch@deepset.ai>
@sjrl
Change StreamingChunk.start to only a bool
665c710
@sjrl
PR comments
9c8ae7b
@sjrl sjrl requested a review from julian-risch June 6, 2025 07:06
julian-risch
julian-risch requested changes Jun 6, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the change requests! Another two small change requests and then the PR is ready to be merged!

Comment thread test/components/generators/chat/test_openai.py Outdated
Comment thread haystack/components/generators/chat/openai.py Outdated
@sjrl sjrl requested a review from julian-risch June 6, 2025 07:31
julian-risch
julian-risch approved these changes Jun 6, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@julian-risch julian-risch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@sjrl sjrl enabled auto-merge (squash) June 6, 2025 08:09
@sjrl sjrl merged commit b61886b into main Jun 6, 2025
22 checks passed
@sjrl sjrl deleted the update-streaming-chunk branch June 6, 2025 08:17
@anakin87 anakin87 mentioned this pull request Jul 22, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@julian-risch julian-risch julian-risch approved these changes

@tstadel tstadel tstadel approved these changes

@dfokina dfokina Awaiting requested review from dfokina dfokina was automatically assigned from deepset-ai/documentation

Assignees

No one assigned

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Extend StreamingChunk to have tool_call, tool_result, start and index

4 participants

@sjrl @coveralls @tstadel @julian-risch